Zoom along a single axis#470
Conversation
voidcenter
force-pushed
the
fix-clampzoom
branch
2 times, most recently
from
678d24f to
a6d5613
Compare
voidcenter
force-pushed
the
fix-clampzoom
branch
from
a6d5613 to
a9dd631
Compare
|
hi @voidcenter can you please update PR title and description? |
|
@danielbarion sure, it has been a while and I updated the description according to my memory. |
|
thanks @voidcenter ! |
| if (this.options.maxWidth !== null && width > this.options.maxWidth) | ||
| { | ||
| const original = this.parent.scale.x; | ||
| console.log('-- maxwidth', this.parent._wheelAxis); |
There was a problem hiding this comment.
can you please remove the console.log?
| if (this.options.minWidth !== null && width < this.options.minWidth) | ||
| { | ||
| const original = this.parent.scale.x; | ||
| console.log('-- minwidth', this.parent._wheelAxis); |
There was a problem hiding this comment.
can you please remove the console.log?
lunarraid
left a comment
lunarraid
left a comment
There was a problem hiding this comment.
We should probably move the _wheelAxis out of the Viewport.ts file entirely and have this entire functionality live solely in the clamp plugin, passing the axis to clamp into the plugin's options.
| width = this.parent.worldScreenWidth; | ||
| height = this.parent.worldScreenHeight; | ||
|
|
||
| if (this.parent._wheelAxis && ['all', 'y'].includes(this.parent._wheelAxis)) { |
There was a problem hiding this comment.
Also switch to simple comparison.
| private _worldHeight?: number | null; | ||
| private _disableOnContextMenu = (e: MouseEvent) => e.preventDefault(); | ||
|
|
||
| public _wheelAxis?: 'all' | 'x' | 'y'; |
There was a problem hiding this comment.
If it's public, why the underscore? Should also make it non-optional and always have a value of one of the three, defaulting to 'all'. Possibly rename to something more generic like scaleAxis since scaling is not solely done by wheel, and wheel is not a global concept in this file but is handled by a plugin.
| width = this.parent.worldScreenWidth; | ||
| height = this.parent.worldScreenHeight; | ||
|
|
||
| if (this.parent._wheelAxis && ['all', 'y'].includes(this.parent._wheelAxis)) { |
There was a problem hiding this comment.
This is needlessly expensive. I'd switch this to a simple comparison.
| { | ||
| super(parent); | ||
| this.options = Object.assign({}, DEFAULT_WHEEL_OPTIONS, options); | ||
| this.parent._wheelAxis = this.options.axis; |
There was a problem hiding this comment.
This does not have to be in the wheel at all as it's not a wheel-specific concept.
3 participants
This change enables zooming along only the x-axis or y-axis.